Conversation
chris-ashe
added
Bootstrap Current
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4091 +/- ##
==========================================
+ Coverage 47.19% 47.41% +0.21%
==========================================
Files 138 139 +1
Lines 29275 29312 +37
==========================================
+ Hits 13817 13897 +80
+ Misses 15458 15415 -43 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
chris-ashe
force-pushed
the
create_bootstrap_current_class
branch
2 times, most recently
from
8717f2a to
e90a20a
Compare
chris-ashe
force-pushed
the
create_bootstrap_current_class
branch
2 times, most recently
from
30b5ca0 to
7683fcb
Compare
chris-ashe
force-pushed
the
create_bootstrap_current_class
branch
2 times, most recently
from
7021d17 to
4f9aa32
Compare
- Modified the Stellarator class to include plasma_bootstrap as a parameter. - Updated the physics fixture in unit tests to instantiate PlasmaBootstrapCurrent. - Refactored multiple test cases to utilize PlasmaBootstrapCurrent for bootstrap fraction calculations. - Ensured all relevant tests in test_physics.py and test_stellarator.py are updated to reflect the new structure.
…onModel and improve error handling
…nd update docstrings for clarity
chris-ashe
force-pushed
the
create_bootstrap_current_class
branch
from
4f9aa32 to
06925e9
Compare
jonmaddock
left a comment
jonmaddock
left a comment
There was a problem hiding this comment.
Great improvement, just some small issues.
process/models/physics/physics.py
Outdated
|
|
||
| class Physics: | ||
| def __init__(self, plasma_profile, current_drive, plasma_beta, plasma_inductance): | ||
| def __init__(self, plasma_profile, current_drive, plasma_beta, plasma_inductance, plasma_bootstrap): |
There was a problem hiding this comment.
As plasma_bootstrap is an instance, can it be given a type hint please?
process/models/physics/physics.py
Outdated
| self.current_drive = current_drive | ||
| self.beta = plasma_beta | ||
| self.inductance = plasma_inductance | ||
| self.bootstrap = plasma_bootstrap |
There was a problem hiding this comment.
bootstrap, plasma_bootstrap or plasma_bootstrap_current? Same names all around please.
process/main.py
Outdated
| ) | ||
| self.plasma_beta = PlasmaBeta() | ||
| self.plasma_inductance = PlasmaInductance() | ||
| self.plasma_bootstrap = PlasmaBootstrapCurrent() |
There was a problem hiding this comment.
Consistent instance naming please: plasma_bootstrap_current.
tests/unit/test_physics.py
Outdated
| """ | ||
|
|
||
| f_c_plasma_bootstrap = physics.bootstrap_fraction_iter89( | ||
| f_c_plasma_bootstrap = PlasmaBootstrapCurrent().bootstrap_fraction_iter89( |
There was a problem hiding this comment.
Perhaps use instance you've just made on physics fixture?
process/models/physics/physics.py
Outdated
| "Illegal value of i_bootstrap_current", | ||
| i_bootstrap_current=physics_variables.i_bootstrap_current, | ||
| ) | ||
| ) from None |
process/models/physics/physics.py
Outdated
| GI_1 = 10 | ||
| GI_2 = 11 | ||
| SUGIYAMA_L_MODE = 12 | ||
| SUGIYAMA_H_MODE = 13 |
| Reference: | ||
| - AEA FUS 172: Physics Assessment for the European Reactor Study, 1989 | ||
| - H. R. Wilson, Nuclear Fusion 32 (1992) 257 | ||
| """Bootstrap current fraction from Wilson et al scaling. |
There was a problem hiding this comment.
Can you use standard reStructuredText (sphinx) docstrings, as in the rest of the code rather than this form (Google or numpy? I forget). We don't want 2 docstring formats in the code. If we've got one, we can then use a tool to convert them to another form if we want.
3 participants
This pull request refactors how the plasma bootstrap current calculations are integrated and tested in the codebase. The main improvement is the explicit use and injection of the
PlasmaBootstrapCurrentclass throughout the main process and test modules, rather than relying on methods attached to thePhysicsclass. This enhances clarity, modularity, and testability of bootstrap current calculations.Core integration and dependency injection:
PlasmaBootstrapCurrentclass is now explicitly instantiated and injected as a dependency into the main process (process/main.py) and theStellaratormodel, ensuring that bootstrap current calculations are managed as a separate, dedicated component.Test suite refactoring:
tests/unit/test_physics.pyhave been updated to directly instantiate and usePlasmaBootstrapCurrent, removing reliance on thePhysicsclass for these calculations. This clarifies test intent and decouples bootstrap current tests from unrelated physics logic.Stellarator model and test updates:
Stellaratormodel and its corresponding test fixture now accept and use thePlasmaBootstrapCurrentclass as a parameter, maintaining consistency with the main process and supporting improved modularity in the stellarator workflow.Checklist
I confirm that I have completed the following checks: